Skip to content

Conversation

@bboynton97
Copy link
Contributor

All logs go to file, then at the end of a trace, the file is uploaded with your API key for storage in S3

@bboynton97 bboynton97 requested review from Dwij1704 and tcdent April 18, 2025 23:58
@codecov
Copy link

codecov bot commented Apr 19, 2025

Codecov Report

Attention: Patch coverage is 61.42857% with 27 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
agentops/client/api/versions/v4.py 5.55% 17 Missing ⚠️
agentops/logging/instrument_logging.py 83.33% 7 Missing ⚠️
agentops/sdk/processors.py 66.66% 2 Missing ⚠️
agentops/sdk/core.py 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Dwij1704
Copy link
Member

The agentops/common directory is currently being used as a shared space for various instrumentation-related functions and utilities. However, logging seems to function as a separate instrumentor in itself. I suggest moving log instrumentation to /agentops/logging cause we might move instrumentation out of agentops for better maintenance, It makes sense to manage and update instrumentation packages independently rather than bumping the sdk itself every time an instrumentation needs update.

@Dwij1704 Dwij1704 linked an issue Apr 21, 2025 that may be closed by this pull request
@bboynton97
Copy link
Contributor Author

The agentops/common directory is currently being used as a shared space for various instrumentation-related functions and utilities. However, logging seems to function as a separate instrumentor in itself. I suggest moving log instrumentation to /agentops/logging cause we might move instrumentation out of agentops for better maintenance, It makes sense to manage and update instrumentation packages independently rather than bumping the sdk itself every time an instrumentation needs update.

I totally understand this sentiment. We're instrumenting print and other loggers in this function so to me it felt like it belonged better in instrumentation, but I see what you're saying with instrumentation packages. i can move it :)

@bboynton97 bboynton97 requested a review from tcdent April 23, 2025 19:07
Copy link
Contributor

@tcdent tcdent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bboynton97 bboynton97 merged commit 0f83678 into main Apr 23, 2025
7 of 10 checks passed
@bboynton97 bboynton97 deleted the s3_logs branch April 23, 2025 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Instrument print

4 participants